Overall Assessment

⚠️ Needs changes

Highest unmitigated severity: P1 on replicate-survey df_survey handling for dCDH inference.

Executive Summary

• I cross-checked the changed methodology against the dCDH registry entry and the survey-design contracts. The new replicate-weight IF expansion and PSU-level Hall-Mammen bootstrap are documented library extensions, so those deviations from the papers/R are not defects by themselves: docs/methodology/REGISTRY.md#L651.
• P1: dCDH’s new _effective_df_survey() helper overwrites the design-level replicate df with min(n_valid)-1 instead of reducing the design df when replicate solves fail. That can make top-level dCDH inference use a df that is too large on rank-deficient replicate designs, and the final value is never written back to results.survey_metadata, so downstream HonestDiD can disagree with the main dCDH surface.
• The new tests only cover clean/full-rank replicate designs (df = R - 1), so they miss both rank-deficient replicate matrices and dropped-replicate paths.
• I could not execute the test suite in this sandbox because Python test deps are unavailable (pytest and numpy are missing), so this is a static review.

Methodology

• Severity P1. Impact: The new helper at diff_diff/chaisemartin_dhaultfoeuille.py#L4980 returns min(n_valid)-1 whenever any replicate-aware SE has run, ignoring the base replicate df contract in diff_diff/survey.py#L590, which is QR-rank of the analysis-weight matrix minus 1 (matching survey::degf()). The registry note also says the effective df is reduced when replicate solves fail, not replaced: docs/methodology/REGISTRY.md#L652. Main dCDH surfaces use this helper at diff_diff/chaisemartin_dhaultfoeuille.py#L1864, while the heterogeneity branch correctly takes min(df_s, n_valid_het - 1) at diff_diff/chaisemartin_dhaultfoeuille.py#L3470 and diff_diff/chaisemartin_dhaultfoeuille.py#L3647. That inconsistency means top-level dCDH p-values/CIs can be anti-conservative on rank-deficient replicate designs. Separately, results are built with the original survey_metadata at diff_diff/chaisemartin_dhaultfoeuille.py#L2768, and HonestDiD later reads survey_metadata.df_survey at diff_diff/honest_did.py#L973, so downstream bounds can use a different df than the main dCDH inference. Concrete fix: make _effective_df_survey() return min(resolved_survey.df_survey, min(n_valid)-1) when both are defined, then persist that final effective df back into survey_metadata.df_survey before constructing ChaisemartinDHaultfoeuilleResults.
• Severity P3. Impact: The core methodology changes themselves are documented in the registry, so I am not flagging the replicate-weight IF expansion or PSU-level Hall-Mammen bootstrap as source-material violations: docs/methodology/REGISTRY.md#L651. Concrete fix: none.

Code Quality

No additional findings beyond the P1 df propagation bug above.

Performance

No findings in the changed code.

Maintainability

No separate maintainability finding. Fixing the authoritative df_survey path will remove the main cross-surface inconsistency.

Tech Debt

No existing TODO entry mitigates the P1 above. The dCDH TODO items at TODO.md#L57 cover placebo SE and parity-test scope, not replicate-df propagation.

Security

No findings.

Documentation/Tests

• Severity P2. Impact: The new tests only assert df_survey == R - 1 on clean replicate designs at tests/test_survey_dcdh_replicate_psu.py#L190 and tests/test_survey_dcdh_replicate_psu.py#L687. They do not exercise either of the two cases the new code is supposed to handle specially: rank-deficient replicate matrices (resolved.df_survey < R-1) or site-level dropped replicates (n_valid < resolved.df_survey + 1). Concrete fix: add one regression with duplicated replicate columns (rank-deficient design) and one with an invalid/dropped replicate column; assert that top-level dCDH inference, results.survey_metadata.df_survey, and HonestDiD all use the same reduced df.
• Severity P3. Impact: _compute_heterogeneity_test()’s docstring still describes the survey path as Binder TSL only at diff_diff/chaisemartin_dhaultfoeuille.py#L3438, but the implementation now dispatches to replicate variance too at diff_diff/chaisemartin_dhaultfoeuille.py#L3634. Concrete fix: update the docstring to mention replicate-weight variance and the effective-df rule.

Path to Approval

1. Change _effective_df_survey() so it never exceeds the base replicate design df from ResolvedSurveyDesign.df_survey; it should only reduce that df when n_valid is smaller.
2. Write the final effective df back into survey_metadata.df_survey before creating ChaisemartinDHaultfoeuilleResults, so honest_did=True, compute_honest_did(results), and exported metadata all consume the same df.
3. Add regression tests for duplicated replicate columns and dropped/invalid replicate columns, and assert consistent df usage across the main dCDH surface and HonestDiD.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 13d5e41f646d39ef2ad9b6c58c3a9542977f8ef9

Overall Assessment
⛔ Blocker

Highest unmitigated severity: P0 on replicate-survey inference when the base replicate design has undefined df_survey (QR rank <= 1).

Executive Summary

• The prior re-review finding is only partially fixed: _effective_df_survey() now caps by the design df and the final value is written back to survey_metadata, so the original top-level rank-deficient df_survey bug is improved on the main path. See diff_diff/chaisemartin_dhaultfoeuille.py:L5009-L5040 and diff_diff/chaisemartin_dhaultfoeuille.py:L2697-L2700.
• P0: dCDH still violates the replicate-df contract when the replicate design’s base df is undefined. _effective_df_survey() returns None, which safe_inference() treats as z-inference, and the new heterogeneity branch can replace an undefined base df with n_valid_het - 1, yielding misleading non-NaN p-values/CIs where the registry requires NaN inference.
• P1: the new heterogeneity integration still bypasses the base design df and can lower survey_metadata.df_survey only after overall_* / event_study_effects inference has already been computed, leaving inconsistent df usage across result surfaces when heterogeneity has the smallest n_valid.
• The replicate-weight IF variance and PSU-level Hall-Mammen bootstrap themselves are documented library extensions in the registry, so those changes are not methodology defects. See docs/methodology/REGISTRY.md:L651-L653.
• I could not execute the tests in this sandbox because pytest and numpy are unavailable.

Methodology

• Severity P0. Impact: the PR newly makes replicate-weight dCDH reachable, but the undefined-df contract is still wrong. The survey registry says replicate df_survey is QR-rank - 1 and is undefined when rank <= 1, which should yield NaN inference, not z-inference. See docs/methodology/REGISTRY.md:L2665-L2670. In dCDH, _effective_df_survey() returns None for that case diff_diff/chaisemartin_dhaultfoeuille.py:L5009-L5040, while safe_inference() treats df=None as normal/z inference and only returns NaN when df <= 0 diff_diff/utils.py:L152-L186. The main dCDH surfaces pass that None straight through, e.g. diff_diff/chaisemartin_dhaultfoeuille.py:L1864-L1867. The new heterogeneity path is even less safe: it starts from resolved.df_survey, but if that is None it explicitly falls back to n_valid_het - 1 diff_diff/chaisemartin_dhaultfoeuille.py:L3499-L3509 and diff_diff/chaisemartin_dhaultfoeuille.py:L3670-L3689. That can silently produce valid-looking p-values/CIs where the registry requires NaN. Concrete fix: keep exported survey_metadata.df_survey=None if that is the project convention, but introduce an internal dCDH inference-df helper that maps “replicate design + undefined effective df” to 0 before every safe_inference() call; in _compute_heterogeneity_test(), never promote an undefined base df to n_valid_het - 1. Add a regression with a rank-1 replicate matrix that still yields finite replicate SEs and assert NaN t_stat/p_value/conf_int on overall, event-study, and heterogeneity outputs.
• Severity P1. Impact: the previous top-level fix is not fully carried through the new heterogeneity integration. _compute_heterogeneity_test() re-derives df_s as min(replicate_n_valid_list) - 1 instead of reusing the base-capped _effective_df_survey() diff_diff/chaisemartin_dhaultfoeuille.py:L3499-L3509, so rank-deficient replicate designs can still get a heterogeneity df that exceeds the design df. Separately, the main dCDH inference surfaces are computed before heterogeneity runs, e.g. diff_diff/chaisemartin_dhaultfoeuille.py:L1628-L1630 and diff_diff/chaisemartin_dhaultfoeuille.py:L1864-L1903, but heterogeneity appends to the shared replicate tracker only later diff_diff/chaisemartin_dhaultfoeuille.py:L2620-L2633, after which survey_metadata.df_survey is overwritten with the smaller final value diff_diff/chaisemartin_dhaultfoeuille.py:L2697-L2700. HonestDiD then reads that final metadata df diff_diff/honest_did.py:L970-L978. So when heterogeneity has the smallest n_valid, overall_* / event_study_effects can be anti-conservative relative to heterogeneity_effects, survey_metadata.df_survey, and HonestDiD. Concrete fix: initialize heterogeneity’s starting df from _effective_df_survey(resolved, replicate_n_valid_list) instead of raw min(n_valid)-1, and finalize the shared replicate df before public inference is frozen, or recompute the already-built dCDH inference fields after heterogeneity updates the tracker.
• Severity P3. Impact: no defect here. The replicate-weight IF expansion and PSU-level Hall-Mammen wild bootstrap are documented deviations from the dCDH papers / R implementation, so I am not treating those changes as source-material mismatches. See docs/methodology/REGISTRY.md:L651-L653. Concrete fix: none.

Code Quality
No additional findings beyond the duplicated / inconsistent df logic already captured above.

Performance
No findings.

Maintainability
No separate maintainability finding beyond the copied df logic in _compute_heterogeneity_test(), which is already the root cause of the P1.

Tech Debt
No separate finding, but the blocking issues above are not mitigated by existing TODO tracking. The dCDH TODO entries currently cover other items only. See TODO.md:L51-L60.

Security
No findings.

Documentation/Tests

• Severity P2. Impact: the new test coverage does not exercise either blocker above. The heterogeneity replicate test only checks finite beta/se tests/test_survey_dcdh_replicate_psu.py:L319-L337, and the new df regressions cover top-level metadata / HonestDiD but do not activate heterogeneity= or the undefined-df (rank <= 1) case tests/test_survey_dcdh_replicate_psu.py:L715-L800. Concrete fix: add one rank-1 replicate design that still produces finite SEs and assert NaN inference on all public dCDH surfaces, plus one rank-deficient replicate design with heterogeneity= asserting consistent df usage across overall_*, event_study_effects, heterogeneity_effects, survey_metadata.df_survey, and HonestDiD.
• Severity P3. Impact: the detailed Phase 3 heterogeneity registry note still describes only TSL survey inference docs/methodology/REGISTRY.md:L618-L618, while the implementation and the top-level dCDH survey note now add a replicate-weight branch diff_diff/chaisemartin_dhaultfoeuille.py:L3454-L3471 and docs/methodology/REGISTRY.md:L651-L653. Concrete fix: update the detailed heterogeneity note so the registry has one consistent survey-inference contract.
• I could not run the added tests here because the sandbox lacks pytest and numpy.

Path to Approval

1. Add a dCDH-local inference-df helper that converts “replicate design with undefined effective df” into df=0 for safe_inference(), and use it at every dCDH inference callsite.
2. In _compute_heterogeneity_test(), stop deriving df_s from raw min(replicate_n_valid_list) - 1; use the same base-capped helper as the rest of dCDH, and never replace an undefined base df with n_valid_het - 1.
3. Make all public dCDH inference surfaces use one final shared replicate df when heterogeneity= is active: either run heterogeneity before the other safe_inference() calls or recompute the already-built public inference fields afterward.
4. Add regression tests for a rank-1 replicate design (finite SE, NaN inference required) and for a rank-deficient design with heterogeneity= asserting df consistency across results and HonestDiD.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 7e62d4cf479d3c4aede4dc595791d2b29fa7b8b1

Overall Assessment

✅ Looks good

Highest unmitigated severity: P2

Executive Summary

• The prior replicate-df blocker appears addressed: replicate designs with undefined base df_survey now route through _inference_df() to force NaN inference instead of silent z-inference, and heterogeneity no longer “rescues” an undefined base df. See diff_diff/chaisemartin_dhaultfoeuille.py#L5079 and diff_diff/chaisemartin_dhaultfoeuille.py#L3731.
• I do not see an unmitigated P0/P1 methodology defect in the new replicate-weight variance or PSU-level bootstrap paths; both are documented library extensions in the Methodology Registry. See docs/methodology/REGISTRY.md#L651.
• P2: the new replicate-placebo regression test is effectively vacuous because it looks up placebo_event_study[1], but the implementation stores placebo horizons under negative keys. That leaves the new replicate placebo branch unverified. See tests/test_survey_dcdh_replicate_psu.py#L268 and diff_diff/chaisemartin_dhaultfoeuille.py#L2388.
• P3: the new registry sentence for replicate HonestDiD df is incomplete. The code uses min(resolved_survey.df_survey, min(n_valid)-1) and preserves undefined base df, but the note states only min(n_valid_across_sites)-1. See docs/methodology/REGISTRY.md#L653 versus diff_diff/chaisemartin_dhaultfoeuille.py#L5109.
• I could not execute the tests in this sandbox because numpy / pandas are unavailable.

Methodology

• Severity P3. Impact: the new registry note says replicate HonestDiD inference uses df_survey = min(n_valid_across_sites) - 1, but the implementation and the detailed heterogeneity note use the capped rule min(resolved_survey.df_survey, min(n_valid) - 1) and keep rank-deficient base designs undefined. That is a documentation mismatch in the project’s methodology source of truth, even though the code path itself looks correct. See docs/methodology/REGISTRY.md#L653, docs/methodology/REGISTRY.md#L618, and diff_diff/chaisemartin_dhaultfoeuille.py#L5109. Concrete fix: update the registry note to the full rule, including the resolved_survey.df_survey cap and the undefined-base-df case.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No findings. The remaining items above are not currently tracked in TODO.md.

Security

No findings.

Documentation/Tests

• Severity P2. Impact: test_placebo_under_replicate() does not actually assert on the replicate placebo result it intends to cover. The implementation stores dynamic placebo horizons under negative keys, but the test queries key 1, so it will silently skip the assertion and pass even if the replicate placebo SE path breaks. See tests/test_survey_dcdh_replicate_psu.py#L268, diff_diff/chaisemartin_dhaultfoeuille.py#L2402, and docs/methodology/REGISTRY.md#L546. Concrete fix: assert on res.placebo_event_study[-1] (or iterate the negative keys), and verify at least se, p_value, and conf_int when n_obs > 0.

I could not run the tests here because the sandbox is missing numpy and pandas.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: b48ee534e3a0934d8767fa9f8c22eb24788ccbd6

Overall Assessment

✅ Looks good

Highest unmitigated severity: P2

Executive Summary

• The prior re-review issues appear addressed: the replicate placebo test now asserts the negative-key convention in tests/test_survey_dcdh_replicate_psu.py#L268, and the HonestDiD replicate-df note now matches the capped implementation in docs/methodology/REGISTRY.md#L653.
• I do not see an unmitigated P0/P1 methodology defect in the new dCDH replicate-weight variance or PSU-level Hall-Mammen bootstrap paths; both are documented library extensions in docs/methodology/REGISTRY.md#L651.
• P2: the new cross-surface replicate-df regression test does not actually exercise the late-arriving min(n_valid) case it is meant to protect, because its duplicated-column setup makes the design-rank cap bind before heterogeneity runs. See tests/test_survey_dcdh_replicate_psu.py#L862 and diff_diff/chaisemartin_dhaultfoeuille.py#L2694.
• P3: the dCDH methodology docs are still internally inconsistent. The section overview still says replicate weights and PSU bootstrap are deferred, while the updated checklist/notes say they are supported. See docs/methodology/REGISTRY.md#L466 versus docs/methodology/REGISTRY.md#L651.
• P3: the updated LLM guide still contains the old “deferred” statement for dCDH survey support. See diff_diff/guides/llms-full.txt#L325.
• I could not execute tests here because pytest is unavailable and the Python environment is missing numpy/pandas.

Methodology

• Severity P3. Impact: the dCDH methodology registry is internally contradictory. The overview in docs/methodology/REGISTRY.md#L466 still says replicate weights and PSU-level bootstrap are deferred, but the supported-contract checklist and notes in docs/methodology/REGISTRY.md#L651 through docs/methodology/REGISTRY.md#L653 now say they are implemented. Because REGISTRY.md is the methodology source of truth, this creates avoidable confusion about whether the PR is a documented extension or an undocumented deviation. Concrete fix: update the overview sentence at line 466 to match the new contract, and keep only the still-deferred single-period placebo-SE limitation called out separately.

Code Quality

• No findings.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings. I did not find these items tracked in TODO.md under “Tech Debt from Code Reviews.”

Security

• No findings.

Documentation/Tests

• Severity P2. Impact: test_heterogeneity_replicate_cross_surface_df_consistency() in tests/test_survey_dcdh_replicate_psu.py#L862 does not create the scenario described by the new recomputation block in diff_diff/chaisemartin_dhaultfoeuille.py#L2694: its duplicated-replicate design makes resolved.df_survey the binding cap before heterogeneity appends anything to _replicate_n_valid_list. That means an incomplete late-propagation bug could still pass the test. Concrete fix: add a targeted regression that monkeypatches compute_replicate_if_variance() to return a smaller n_valid on a later IF site and then assert the final reduced df propagates to all copied public surfaces, not just survey_metadata and the heterogeneity dict.
• Severity P3. Impact: the user-facing guide still says dCDH replicate weights and PSU bootstrap are deferred in diff_diff/guides/llms-full.txt#L321 through diff_diff/guides/llms-full.txt#L325, which now contradicts both the updated signature note in diff_diff/guides/llms-full.txt#L264 through diff_diff/guides/llms-full.txt#L269 and the survey feature summary in diff_diff/guides/llms-full.txt#L1673 through diff_diff/guides/llms-full.txt#L1676. Concrete fix: update the stale note at line 325 to reflect the new support matrix.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: bcc14c41e18ac4a09ce36f87e044857c7e829250

Overall Assessment

⛔ Blocker

Highest unmitigated severity: P0

Executive Summary

• Prior re-review doc drift on dCDH survey support appears resolved: the methodology registry now documents replicate-weight variance and PSU-level Hall-Mammen bootstrap as supported library extensions in docs/methodology/REGISTRY.md:L466 and docs/methodology/REGISTRY.md:L651.
• I do not see an undocumented methodology deviation in the new replicate-weight variance or PSU-level bootstrap mechanics themselves; the main issue is an inference-propagation bug on a public results surface.
• Severity P0: the new late-df_survey recompute path updates overall/event-study/placebo/heterogeneity outputs, but it does not update results.normalized_effects. When heterogeneity= contributes the binding replicate n_valid, normalized-effects inference can disagree with the final survey_metadata.df_survey and silently report wrong p_value/conf_int (and potentially miss the all-NaN gate when the final replicate df becomes undefined).
• The new regression test that claims to cover “every public surface” omits normalized_effects, which is why this bug can pass the new suite.
• I could not execute pytest here because pytest is not installed in this environment.

Methodology

Affected methods checked: dCDH survey IF expansion / replicate-weight analytical variance, heterogeneity under replicate weights, and PSU-level Hall-Mammen wild bootstrap. The new extensions are documented in docs/methodology/REGISTRY.md:L466, docs/methodology/REGISTRY.md:L618, and docs/methodology/REGISTRY.md:L651. I did not find an undocumented paper/registry mismatch in those mechanics.

• Severity P0. Impact: normalized_effects inference is computed with the pre-heterogeneity _df_survey in diff_diff/chaisemartin_dhaultfoeuille.py:L2472, while heterogeneity can append a smaller replicate n_valid later in diff_diff/chaisemartin_dhaultfoeuille.py:L2628. The final recompute block added to make “every surface” agree starts at diff_diff/chaisemartin_dhaultfoeuille.py:L2694, but it never revisits normalized_effects, and the stale dict is returned at diff_diff/chaisemartin_dhaultfoeuille.py:L2836. That is silent wrong statistical output on a public surface. It is especially bad when the final replicate df becomes undefined, because the project’s NaN contract is to force all inference fields to NaN via diff_diff/chaisemartin_dhaultfoeuille.py:L5107 and diff_diff/utils.py:L152, but normalized_effects would still keep the earlier finite values. Concrete fix: recompute normalized_effects_out[h]["t_stat"], p_value, and conf_int with _final_inf_df inside the post-heterogeneity block, or move normalized-effects inference construction until after _final_eff_df is finalized.

Code Quality

• No findings.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings. The blocker above is not mitigated by tracking; I did not find a matching entry under TODO.md:L51.

Security

• No findings.

Documentation/Tests

• Severity P2. Impact: the new late-n_valid regression explicitly says it covers “every public surface” but only asserts overall, event-study, placebo, heterogeneity, and survey_metadata.df_survey in tests/test_survey_dcdh_replicate_psu.py:L922 and tests/test_survey_dcdh_replicate_psu.py:L936. normalized_effects is also a public surface, including export via diff_diff/chaisemartin_dhaultfoeuille_results.py:L1216. That gap is why the P0 above can slip through. Concrete fix: extend test_heterogeneity_late_nvalid_propagates_to_all_surfaces() to assert res.normalized_effects[h]["p_value"] and conf_int use the final reduced df; add a second case with reduced_n_valid=1 to verify the all-NaN branch.
• I could not run the test suite here because pytest is unavailable.

Path to Approval

1. Update the final replicate-df propagation block in diff_diff/chaisemartin_dhaultfoeuille.py:L2694 so normalized_effects_out is recomputed with _final_inf_df, or defer building normalized-effects inference until after _final_eff_df is known.
2. Extend tests/test_survey_dcdh_replicate_psu.py:L922 to cover results.normalized_effects, including a case where late heterogeneity reduces the final df and a case where it makes the final replicate df undefined (n_valid < 2).

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: abdfea4630e86fd7079870ee3602696871d2068f

Overall Assessment

✅ Looks good

Highest unmitigated severity: P3

Executive Summary

• The prior blocker on replicate-df_survey propagation appears resolved: the final recompute block now updates every affected public surface, including normalized_effects, and persists the final survey df in diff_diff/chaisemartin_dhaultfoeuille.py:L2694-L2801. The new regressions explicitly cover normalized_effects and the all-NaN branch in tests/test_survey_dcdh_replicate_psu.py:L996-L1166.
• I do not see an undocumented methodology deviation in the new replicate-weight analytical variance, heterogeneity IF path, or PSU-level bootstrap. Those extensions are now documented in docs/methodology/REGISTRY.md:L618-L618 and docs/methodology/REGISTRY.md:L651-L653.
• Severity P3: the new coarser-PSU warning is computed from raw positive-weight groups, not the post-filter eligible-group set actually used by the bootstrap map, so filtered panels can still get a stale “PSU coarser than group” warning in diff_diff/chaisemartin_dhaultfoeuille.py:L1973-L1990 and diff_diff/chaisemartin_dhaultfoeuille.py:L2145-L2165.
• Severity P3: two changed-file docstrings still describe the bootstrap as group-clustered only, which is now incomplete after adding the PSU-level survey path in diff_diff/chaisemartin_dhaultfoeuille.py:L302-L310 and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L5-L10.
• I could not run pytest here because this environment is missing pytest, numpy, and pandas. I did verify the changed Python files parse successfully.

Methodology

• No unmitigated findings. The affected methods are dCDH survey IF expansion / replicate-weight variance, heterogeneity testing under survey weights, and PSU-level Hall-Mammen bootstrap; the new behavior is documented as a library extension in docs/methodology/REGISTRY.md:L618-L618 and docs/methodology/REGISTRY.md:L651-L653, and the previous cross-surface df inconsistency appears fixed in diff_diff/chaisemartin_dhaultfoeuille.py:L2694-L2801.

Code Quality

• Severity P3. Impact: the new warning gate uses all positive-weight raw groups, while the actual PSU bootstrap map is built from post-filter eligible_group_ids; if upstream dCDH filtering drops groups, the code can still emit a warning even when the effective bootstrap target is one group per PSU. Concrete fix: compute the warning counts from eligible_group_ids_bootstrap / group_id_to_psu_code_bootstrap (or from the same post-filter mask used to build them), and add a regression with dropped groups sharing PSUs. See diff_diff/chaisemartin_dhaultfoeuille.py:L1973-L1990 and diff_diff/chaisemartin_dhaultfoeuille.py:L2145-L2165.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings. I did not find a matching tracked limitation under the “Tech Debt from Code Reviews” section of TODO.md:L51.

Security

• No findings.

Documentation/Tests

• Severity P3. Impact: two in-code docstrings still say dCDH’s bootstrap is clustered at the group level only, which no longer matches the implemented survey PSU path and can mislead future readers about the supported contract. Concrete fix: update both docstrings to say “group-level by default; PSU-level under survey_design when PSUs are coarser than groups,” and point readers to the registry note. See diff_diff/chaisemartin_dhaultfoeuille.py:L302-L310 and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L5-L10.
• No additional test-gap findings in the re-review scope. The new regression coverage now exercises the previous normalized_effects gap and the final-NaN path in tests/test_survey_dcdh_replicate_psu.py:L996-L1166. I could not execute the suite in this environment.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 41474069e87331a1019a4177666ca4cb52b80a9f

Overall Assessment

Looks good

Highest unmitigated severity: P3

Executive Summary

• Previous re-review concerns around replicate-df_survey propagation and the stale coarser-PSU warning appear resolved in diff_diff/chaisemartin_dhaultfoeuille.py:1971 and diff_diff/chaisemartin_dhaultfoeuille.py:2714.
• The PR affects dCDH survey IF variance, survey heterogeneity SEs, PSU-level multiplier bootstrap, and survey-backed twowayfeweights(). I do not see an unmitigated P0/P1 methodology mismatch relative to the dCDH papers plus the documented library extensions in docs/methodology/REGISTRY.md:651, docs/methodology/REGISTRY.md:652, and docs/methodology/REGISTRY.md:653.
• Severity P3: some registry/class/user-facing docs still say dCDH “always clusters at the group level,” which now conflicts with the implemented survey-bootstrap contract.
• Severity P3: diff_diff/guides/llms-full.txt:321 still says placebo bootstrap is deferred and that bootstrap only covers DID_M, DID_+, and DID_-, which is stale after this PR’s L_max >= 1 placebo/event-study support.
• Static review only. I could not run pytest here because this environment is missing numpy, pandas, and pytest.

Methodology

• No unmitigated findings. The affected methods are the dCDH survey IF expansion / analytical variance path, survey heterogeneity regression inference, PSU-level Hall-Mammen wild bootstrap, and survey-backed TWFE aggregation in diff_diff/chaisemartin_dhaultfoeuille.py:3540, diff_diff/chaisemartin_dhaultfoeuille.py:5140, diff_diff/chaisemartin_dhaultfoeuille.py:5238, and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:68. Those extensions are explicitly documented in the registry, so I’m not treating them as methodology defects.

Code Quality

• No findings.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No new untracked findings. I did not see a silent-correctness issue being deferred, and I did not find a matching blocker under TODO.md:51.

Security

• No findings.

Documentation/Tests

• Severity P3. Impact: the methodology docs and user-facing cluster text are now internally inconsistent. The new contract is documented at docs/methodology/REGISTRY.md:653, but older text still says dCDH “always clusters at the group level” in docs/methodology/REGISTRY.md:592, docs/methodology/REGISTRY.md:636, diff_diff/chaisemartin_dhaultfoeuille.py:321, diff_diff/chaisemartin_dhaultfoeuille.py:432, and diff_diff/chaisemartin_dhaultfoeuille.py:503. Concrete fix: update the stale note/checklist item, constructor docstring, and cluster error messages to distinguish unsupported user-specified cluster= from automatic PSU-level clustering under survey_design.
• Severity P3. Impact: diff_diff/guides/llms-full.txt:321 still says Phase 2 will add placebo bootstrap support and that bootstrap only covers DID_M, DID_+, and DID_-, but the PR now bootstraps multi-horizon event-study and placebo horizons via diff_diff/chaisemartin_dhaultfoeuille.py:2041, diff_diff/chaisemartin_dhaultfoeuille.py:2086, diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:284, and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:369. Concrete fix: rewrite that note so it says only the single-period DID_M^pl (L_max=None) remains NaN; L_max >= 1 placebos and event-study horizons have analytical/bootstrap inference.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 9368ae65b89c347e469c9d711607e50acf434a5a

Overall Assessment

Looks good

Highest unmitigated severity: P3

Executive Summary

• Previous re-review concerns around replicate df_survey propagation and the stale coarser-PSU bootstrap warning appear resolved in diff_diff/chaisemartin_dhaultfoeuille.py:L1836-L1909, diff_diff/chaisemartin_dhaultfoeuille.py:L1949-L2023, and diff_diff/chaisemartin_dhaultfoeuille.py:L2718-L2821.
• The PR affects dCDH survey IF/replicate variance, survey heterogeneity SEs, PSU-level Hall-Mammen bootstrap, and survey-backed twowayfeweights(). Against the dCDH papers plus the documented registry extensions, I do not see an unmitigated P0/P1 methodology defect in the changed math or variance handling at docs/methodology/REGISTRY.md:L651-L653, diff_diff/chaisemartin_dhaultfoeuille.py:L3540-L3818, and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L68-L399.
• Severity P3: the cluster-contract text is still internally inconsistent. New registry text documents automatic PSU-level bootstrap under survey_design, but older user-facing text still says dCDH “always clusters at the group level.”
• Severity P3: the LLM guide still says placebo bootstrap is deferred and only covers DID_M, DID_+, and DID_-, which conflicts with the implemented multi-horizon placebo/event-study path.
• Static review only. I could not run tests here because the environment lacks numpy, pandas, scipy, and pytest.

Methodology

No findings. Affected methods are the dCDH survey IF/replicate variance path on the main ATT, event-study, and placebo surfaces diff_diff/chaisemartin_dhaultfoeuille.py:L1718-L1768, diff_diff/chaisemartin_dhaultfoeuille.py:L1836-L1909, the survey heterogeneity path diff_diff/chaisemartin_dhaultfoeuille.py:L3540-L3818, PSU-level Hall-Mammen bootstrap diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L68-L399, and survey-backed twowayfeweights() aggregation diff_diff/chaisemartin_dhaultfoeuille.py:L5604-L5694. These are explicitly documented as library extensions in docs/methodology/REGISTRY.md:L618-L618 and docs/methodology/REGISTRY.md:L651-L653, and the code routes through the shared survey helpers consistently.

Code Quality

No findings.

Performance

No findings.

Maintainability

No findings.

Tech Debt

No findings. I did not find a silent-correctness issue being deferred via TODO.md:L51-L83.

Security

No findings.

Documentation/Tests

• Severity: P3. Impact: the cluster contract is still described inconsistently. The newer survey/bootstrap note says automatic PSU-level Hall-Mammen clustering can occur under survey_design docs/methodology/REGISTRY.md:L651-L653, but older text still says dCDH “always clusters at the group level” in the constructor docstring diff_diff/chaisemartin_dhaultfoeuille.py:L321-L329, both NotImplementedError messages diff_diff/chaisemartin_dhaultfoeuille.py:L432-L442 and diff_diff/chaisemartin_dhaultfoeuille.py:L503-L510, the older registry note docs/methodology/REGISTRY.md:L592-L592, and the mirrored test description/regex tests/test_chaisemartin_dhaultfoeuille.py:L405-L425. Concrete fix: update those stale strings to distinguish unsupported user-specified cluster= from automatic PSU-level bootstrap under survey designs, and align the test wording with that contract.
• Severity: P3. Impact: the guide still says placebo bootstrap support is deferred and only covers DID_M, DID_+, and DID_- diff_diff/guides/llms-full.txt:L321-L325, which conflicts with the implemented multi-horizon placebo analytical path diff_diff/chaisemartin_dhaultfoeuille.py:L1718-L1768 and bootstrap path diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L369-L397. Concrete fix: rewrite that note so only single-period DID_M^pl (L_max=None) is called out as NaN, while L_max >= 1 placebos and event-study horizons are described as having analytical/bootstrap inference.

/ai-review

🔁 AI review rerun (requested by @igerber)

Head SHA: 0e537b0caefb85db7ff525ebf0d01e11ab70546e

Overall Assessment

✅ Looks good

Executive Summary

• Previous re-review P3s around the dCDH cluster-contract wording and the placebo/bootstrap guide text appear resolved in diff_diff/chaisemartin_dhaultfoeuille.py:L334-L355, docs/methodology/REGISTRY.md:L592-L653, diff_diff/guides/llms-full.txt:L321-L325, and tests/test_chaisemartin_dhaultfoeuille.py:L405-L431.
• The methodology-sensitive changes are concentrated in the dCDH survey/replicate SE dispatch, heterogeneity survey variance path, PSU-level bootstrap clustering, and survey-backed twowayfeweights() parity at diff_diff/chaisemartin_dhaultfoeuille.py:L638-L827, diff_diff/chaisemartin_dhaultfoeuille.py:L1648-L2055, diff_diff/chaisemartin_dhaultfoeuille.py:L2165-L2851, diff_diff/chaisemartin_dhaultfoeuille.py:L3570-L3858, and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L68-L588.
• Against the cited dCDH source material, the core estimator still sits on the paper’s group-level asymptotic-normality / normal-approximation CI setup; the survey IF expansion, replicate-weight variance, and PSU-level bootstrap are now clearly documented as library extensions rather than paper-parity claims. (nber.org)
• I do not see an unmitigated P0/P1 in the changed math, variance/SE handling, df propagation, or cluster/control logic.
• The added regression coverage is targeted and materially useful: replicate SE across Class A/Class B paths, coarser-PSU bootstrap invariants, rank-deficient replicate-df capping, and NaN inference when replicate df is undefined are all exercised in tests/test_survey_dcdh_replicate_psu.py:L150-L1148 and tests/test_survey_dcdh.py:L320-L409.
• Static review only: this environment does not have numpy, pandas, scipy, or pytest, so I could not execute the suite.

Methodology

• No findings. The changed estimator paths in diff_diff/chaisemartin_dhaultfoeuille.py:L1648-L2055, diff_diff/chaisemartin_dhaultfoeuille.py:L3570-L3858, diff_diff/chaisemartin_dhaultfoeuille.py:L5170-L5374, and diff_diff/chaisemartin_dhaultfoeuille_bootstrap.py:L68-L588 match the updated registry contract in docs/methodology/REGISTRY.md:L592-L653. The dynamic companion paper’s inference section remains an independent-groups, normal-approximation result; this PR’s survey/replicate/PSU-bootstrap behavior is therefore appropriately treated as documented library extension rather than undocumented deviation. (nber.org)

Code Quality

• No findings.

Performance

• No findings.

Maintainability

• No findings.

Tech Debt

• No findings.

Security

• No findings.

Documentation/Tests

• No findings. The changed public-facing contract is now internally consistent across the estimator docstring, cluster gate, registry notes, and AI guide text in diff_diff/chaisemartin_dhaultfoeuille.py:L334-L355, docs/methodology/REGISTRY.md:L592-L653, diff_diff/guides/llms-full.txt:L321-L325, and tests/test_chaisemartin_dhaultfoeuille.py:L405-L431.
• Validation note: I could not run the tests here because the environment is missing numpy, pandas, scipy, and pytest; this review is based on static inspection of tests/test_survey_dcdh_replicate_psu.py:L150-L1148, tests/test_survey_dcdh.py:L320-L409, and tests/test_survey_dcdh.py:L761-L785.